-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Sea 178 test actions loading #110
Conversation
…not cal_adjust, remove debug messages
…_sensor_calibration_data
…object in capture
…A-178_test_actions_loading
@@ -1,26 +1,22 @@ | |||
nasctn_sea_data_product: | |||
name: test_nasctn_sea_data_product | |||
name: test_SEA_CBRS_Measure_Baseline | |||
rf_path: antenna |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this same rf_path: antenna param should be added to all of the test actions to allow them to work with a configurable preselector or without a preselector. Currently, they will all fail on a sensor if it has a configurable preselector.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 8d205e5
…A-178_test_actions_loading
scos_actions/hardware/sensor.py
Outdated
@@ -246,6 +246,26 @@ def recompute_calibration_data(self, params: dict) -> None: | |||
if not recomputed: | |||
logger.warning("Failed to recompute calibration data") | |||
|
|||
def check_sensor_overload(self, data) -> bool: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like data is expected to be an ndarray? I suggest adding a type hint. Also, is there any danger that data could not be cast to np.complex64? I'm not sure.
10 * np.log10(1 / (2 * 50)) + 30 | ||
) # Convert log(V^2) to dBm | ||
# explicitly check is not None since 1db compression could be 0 | ||
if self.sensor_calibration_data["compression_point"] is not None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggest moving this above so you don't compute the avg power if you don't know the compression point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like this is only called if there is compression_point so maybe the previous comment can be disregarded unless there is a chance check_sensor_overload could be called in a different flow.
scos_actions/settings.py
Outdated
@@ -26,6 +26,10 @@ | |||
logger.debug(f"scos-actions: RUNNING_TESTS:{RUNNING_TESTS}") | |||
FQDN = env("FQDN", None) | |||
logger.debug(f"scos-actions: FQDN:{FQDN}") | |||
SIGAN_MODULE = env.str("SIGAN_MODULE", default="scos_actions.hardware.mocks.mock_sigan") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I would only set the defaults for these when running tests.
scos_actions/discover/__init__.py
Outdated
), | ||
"logger": Logger(), | ||
} | ||
actions = {"logger": Logger()} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we remove logger from the actions? Not a big deal either way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. See comments for a couple minor suggestions.
added sigan to mock GPS constructor just like the scos-usrp USRPLocation class